Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed config file being overriden when it's blocked by another application. #1236

Closed
wants to merge 1 commit into from

Conversation

drfrag666
Copy link
Contributor

No description provided.

src/common/utility/configfile.cpp Outdated Show resolved Hide resolved
src/common/utility/configfile.cpp Outdated Show resolved Hide resolved
src/common/utility/configfile.cpp Outdated Show resolved Hide resolved
@drfrag666
Copy link
Contributor Author

Done.

@Doom2fan Doom2fan assigned Doom2fan and unassigned Doom2fan Nov 18, 2020
@alexey-lysiuk
Copy link
Collaborator

alexey-lysiuk commented Nov 18, 2020

All speculations about this issue revolve around inability to read existing config file at some point for an unknown reason.
Did you test this change in the given situation?

When I removed all permissions from config file, FileExists() returns false because _wstat64() returns -1.
So, the added code doesn't handle the advertised error condition, at least on Windows.

@drfrag666
Copy link
Contributor Author

No but _stat is valid in order to check for file existence in theory. In case it returns -1 it's becouse the file doesn't exist or a parameter is not valid. I wouldn't be surprised if it's another bug in _stat. I could try to force the internal version in LZDoom, i'm using VS 2017 BTW. On the other side ::exits internally uses GetFileAttributesW.

@alexey-lysiuk
Copy link
Collaborator

This is not what I was asking about. I see no point in change that doesn't fix the problem even if assumptions are correct. The latter wasn't proved, but this doesn't matter if the added code are not called anyway.
To put it clear, a theoretical fix for a theoretical problem is not how things work.

@drfrag666
Copy link
Contributor Author

drfrag666 commented Nov 18, 2020

The problem is real, what happens when you remove the permissions from the file? I assume the config is still overriden, i will do some testing in that situation then to see if it's a bug in _stat or ::exists work. Well we would need to change the permissions again before exiting of course.

@drfrag666
Copy link
Contributor Author

Well, i've just confirmed that _stat is broken on windows and the internal version works. I see it's still in GZDoom guarded by #ifdef USING_V110_SDK71 and it's _wstat64i32 but now _wstat64 is used instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants